Compose shared dialogs, tooltips and navigation - #87

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Reviewed 338bbb5be64cb9bcd312ca153cc49ce402f0332f against stacked base b31a9a8d93352569ba2a7f49ad2c584e16ce5af4, not main.

[P2] Preserve the tooltip description after render-element prop merging

src/shared/design-system/ui/Tooltip.tsx:17–23

When the child already has aria-describedby, the combined value passed to BaseTooltip.Trigger is overwritten by the original child prop in render={children}. Base UI 1.7.0 merges render.props last. As a result, the tooltip opens visibly but its text is absent from the control’s accessible description, violating this wrapper’s description-link contract. This is a defect in the new shared component’s supported composition, not a regression in an existing app screen; exact-head adoption is limited to the viewer/tests.

Reproduced in Chromium and WebKit using the real shared components: <Tooltip content="Additional hint"><Button aria-describedby="existing-help">Described action</Button></Tooltip> plus the existing help element. Focus the button; the tooltip opens with an ID, but the button retains only aria-describedby="existing-help" for the full assertion timeout. The same probe without an existing description passes in both engines.

Apply the combined description on the final rendered trigger, preserving the original IDs. Add a mounted regression asserting both descriptions while open and only the original after dismissal; retain the control’s accessible name.

Focused browser validation also passed modal shortcut isolation, allowed-in-modal dispatch, initial focus, focus wrapping, pending Escape/outside-press protection, dismissal, and keyboard return focus in both engines. No broad CI-equivalent suites or native visual checks rerun locally. This review does not certify the later #88 app adoption.

Non-blocking catalog gap: NavigationItem adds variant="pill", but ui/registry.ts:435–445 and tests/fixtures/design-system/ui/componentSpecimens.tsx:624–661 still document/exercise only rows. Add the pill’s rest/selected specimen and update variant metadata; no broken pill behavior was established.

All three independent source-review lanes returned. No additional material code defects were identified in the reviewed Dialog/overlay, Tabs, navigation forwarding, Field or history-test changes. Current hosted checks pass; full local scan and native/human visual acceptance remain deferred as documented by the author.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear at this head; prior tooltip blocker resolved

Reviewed 1b5a4fcf597b9a44fd067e1ed1a3c3cde83aada8 against exact stacked base afc4de9a603178442aa4f16d0e884043586cfe43 (PR #86), not main. This is a COMMENTED re-review, not approval.

• Tooltip description composition is corrected. src/shared/design-system/ui/Tooltip.tsx:20–30 applies the combined aria-describedby to the final render element via cloneElement, so the child’s original IDs no longer overwrite the added tooltip reference. The mounted regression at compositions.test.tsx:153–188 asserts two retained descriptions plus the live tooltip ID while open, the unchanged accessible name, and only the original descriptions after Escape.
• The NavigationItem catalog gap is closed. Registry metadata and the interactive specimen now include the supported pill variant, with its existing shared styling. Dialog and RadioGroup examples survive the parent refresh.
• No additional blocking defect was established in the integration delta. Source review covered the changed registry/specimen sets, overlay tokens and portal scope, Dialog ownership, tabs/panels, navigation forwarding and merge preservation. Unchanged accepted paths were compared against the earlier reviewed source rather than broadly re-reviewed.

Evidence and limits

Both complementary source-review lanes completed on a verified bare Blox object store. Existing hosted checks are 12/12 successful for this exact head. No repository code was checked out, installed, built, tested, imported or executed during this review. The tooltip lifecycle evidence here is source and mounted-test coverage plus existing CI, not a new browser/accessibility-tree measurement. Base UI package implementation was not independently inspected.

PR #86’s legacy-radius finding and PR #73’s fixed-height settings finding are inherited parent concerns, not newly introduced #87 defects. This review does not certify assembled-stack desktop acceptance or #88 adoption, dismiss earlier reviews, approve, or merge.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no actionable blockers found for head 3b276793123aea43c23767d8c93f4e262b2a5872 against base 0beb523443557c189e5fcf16a3ac9e8437666690. This is a COMMENTED review, not approval or merge authorization.

• Convergent re-review: the current 13-file, +575/-19 composition patch has the same stable patch ID as the previously reviewed delta at 1b5a4fcf597b9a44fd067e1ed1a3c3cde83aada8. Source comparison preserves main’s changes across all 140 changed paths, aside from the intended additive design documentation and two popover layer substitutions. Tooltip description/open/Escape handling, Dialog ownership, Tabs’ existing no-panel branch, navigation forwarding and registry/specimen coverage remain intact.
• Contract and scope: Block UI supplies the visual language, Base UI owns interaction, and Buzz owns composition/styles with the host remaining the sole appearance/reset owner. The new overlay stylesheet is wired into the viewer but not the shipping host. At this head, the new Dialog/Tooltip, panel-tab and pill usages are confined to tests/viewer; the registry is documentation metadata, not a component export. That is a downstream adoption integration requirement, not a demonstrated current-head defect. This review does not certify #88 adoption.
• Evidence and limits: two bounded source lanes and coordinator verification ran read-only on the pinned Blox object store. Existing exact-head CI shows 11 successful checks, including CI required, and one skipped Windows native validation check. No checkout, install, build, test or PR-code execution was performed for this review. Runtime geometry, outside-pointer dismissal and attended native/visual acceptance were not newly exercised; Base UI internals were not inspected.